WP/GetMetaSingle: simplify code using two class constants #2631
Conversation
rodrigoprimo
force-pushed
the
update-get-meta-single
branch
from
00496d4 to
2d204ca
Compare
Now that support for PHP < 7.2 has been dropped in PR 2614, we can simplify the sniff code by defining the two different function signature formats as class constants. This was not possible before as PHP < 5.6 did not support defining class constants as arrays.
rodrigoprimo
force-pushed
the
update-get-meta-single
branch
from
2d204ca to
e331fe6
Compare
rodrigoprimo
force-pushed
the
update-get-meta-single
branch
from
2f9ae1b to
a09975d
Compare
There was a problem hiding this comment.
Thanks @rodrigoprimo !
Only remark I have left is that as support for PHP < 7.2 has been dropped, these new class constants could be defined with visibility and in this case, I'd recommend private visibility as the info is only relevant to this sniff.
Not a blocker though, as the pre-existing constants don't have visibility modifiers yet, so I expect that to be changed in a future PR anyway.
Only thing to remember about this is that the pre-existing constants must be declared as public and the visibility can't be changed until WPCS 4.0, while for new class constants like these, we can use the correct visibility from the get-go.
|
Thanks, @jrfnl! Defining the visibility of the two new constants to Sorry @dingo-d for pushing the change right after you submitted another review for this PR. |
3 participants
Description
Now that support for PHP < 7.2 has been dropped in PR #2614, we can simplify the sniff code by defining the two different function signature formats as class constants. This was not possible before, as PHP < 5.6 did not support defining class constants as arrays.
Suggested changelog entry
N/A